Conversation
Scanning the task table told you a task's state and whose turn it was, but not whether it had a plan to read or a PR to open — you had to press `a` or `p` on each row to find out. Add a two-slot marks column, left of the name it annotates: `❏` when the task has an unhidden artifact, `➚` when it has a url. The header doubles as the legend. Artifacts are files rather than task columns, so presence has to ride along on the list response or the dashboard would need a request per row on every refresh. `GET /tasks` now reports `has_artifacts`, resolved once per response through a new bulk `ArtifactStore.tasks_with_artifacts`. It is concrete on the ABC (defaulting to `list()` per id, like `link_slug` defaults to a no-op), with the filesystem store overriding it with a single-pass `os.scandir` that stops at the first unhidden entry. Writing an artifact now bumps the change feed too — it stores no row of its own, so without that a newly written plan would not wake a parked long-poll and the mark would lag until an unrelated mutation. The dotfile rule that decides "unhidden" moves to `core.artifacts.is_hidden`, shared with the dashboard's existing "Show hidden" toggle. `❏` (U+274F) and `➚` (U+279A) rather than 📁/🔗 because both are East_Asian_Width=Neutral with no emoji presentation form: exactly one cell in every terminal. Emoji are Width=Wide, which would make the column's rendered width depend on the viewer's terminal and font. A test pins equal cell width across all four mark combinations to keep a later edit from regressing that. Tests that read row cells positionally now resolve the index by column label, so inserting a column doesn't renumber assertions across the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`❏➚` read as one glyph cluster at a glance. A space between the slots makes each mark legible on its own; the cell is now three cells wide (mark, gap, mark) and still constant whichever marks a row carries. The header and the cell format share one `_MARKS_LABEL` definition, so the legend can't drift from what the rows render. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`tasks_with_artifacts(ids) -> set` was the only plural verb on a store whose every other method (`get`, `list`, `put`, `link_slug`) takes one task id, and it coupled the caller to passing ids it had just read out of the store. Replace it with `has_unhidden_artifacts(task_id) -> bool`, so the interface is uniform and a future single-task caller — `has_artifacts` on the task detail, say — reads naturally instead of going through a one-element list. The cost is a thread hop per row rather than one per response: ~19ms against ~3ms for 50 tasks, all of it hop overhead rather than filesystem work. That is well inside the noise for a TUI refresh at dashboard scale, and the uniform interface is worth it. Revisit if a fleet ever grows large enough to feel it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tildesrc
commented
Sep 15, 2026
| lambda: sorted(p.name for p in task_dir.iterdir() if p.is_file()) | ||
| ) | ||
|
|
||
| def _has_artifacts_sync(self, task_id: str) -> bool: |
Contributor
Author
There was a problem hiding this comment.
Why is this synchronous?
Contributor
Author
There was a problem hiding this comment.
Because it's blocking filesystem I/O, and none of that may run on the event loop — os.scandir (like the iterdir/read_bytes/is_dir calls in the methods above) blocks the calling thread. So the sync function is the body that has_unhidden_artifacts hands to a worker thread:
return await asyncio.to_thread(self._has_artifacts_sync, task_id)The reason it's a separate named method, rather than the inline lambda the sibling methods pass to to_thread, is that this body needs with and try/except — neither fits in a lambda. _link_slug_sync/link_slug is split for the same reason.
Pushed fe6f338 to say that in the docstring, since the question was fair: it explained the scandir choice but never the sync/async split.
The docstring explained the scandir choice but not the sync/async split, so the obvious review question — why isn't this just async? — had no answer in the code. Record it: it's blocking filesystem I/O handed to a worker thread by `has_unhidden_artifacts`, and it's a named method rather than the inline lambda the sibling methods use because it needs `with` and `try`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scanning the task table told you a task's state and whose turn it was, but not whether it had a plan to read or a PR to open — you had to press
aorpon each row to find out.This adds a two-slot marks column, left of the name it annotates:
❏when the task has an unhidden artifact,➚when it has a url. The header doubles as the legend.Getting the bit to the dashboard
Artifacts are files rather than task columns, so presence has to ride along on the list response or the dashboard would need a request per row on every refresh.
GET /tasksnow reportshas_artifacts, resolved through a newArtifactStore.has_unhidden_artifacts(task_id). It's concrete on the ABC — defaulting to a scan oflist(), the waylink_slugdefaults to a no-op — so no adapter breaks by not knowing about it, and the filesystem store overrides it with anos.scandirthat stops at the first unhidden entry.Writing an artifact now bumps the change feed too. An artifact stores no row of its own, so without that a newly written
plan.mdwouldn't wake a parked long-poll and the mark would lag until some unrelated mutation came along.The dotfile rule that decides "unhidden" moves to
core.artifacts.is_hidden, shared with the dashboard's existing "Show hidden" toggle rather than spelled out at each surface.Why these glyphs
❏(U+274F) and➚(U+279A) rather than 📁/🔗: both areEast_Asian_Width=Neutralwith no emoji presentation form, so they occupy exactly one cell in every terminal. Emoji areWidth=Wide, which makes the rendered column width depend on the viewer's terminal and font. A test pins equal cell width across all four mark combinations, so a later edit can't quietly regress it. The two marks are separated by a space, making the cell three cells wide.Notes for the reviewer
has_artifactsis on the summary shape only, notTaskOut._task_outis sync and called from ~20 mutation handlers; making it async to add a listdir per mutation response isn't worth it for a field only the table needs, andGET /tasks/{id}/artifactsalready answers it exactly for one task.Ambiguous-width glyphs (the│ ├─ └─connectors, the–status placeholder, the snoozed·), which do drift in a CJK-configured terminal. Independent of this change and worth its own decision.Plan: the task's
plan.mdartifact.